Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Swift 5.7 as the minimum supported version #146

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

3a4oT
Copy link

@3a4oT 3a4oT commented Aug 29, 2024

  • Set Swift 5.7 as the minimum supported version.
  • Removed CI job for Ubuntu 20.04.
  • Bumped swift-nio and swift-collections versions on the manifest.

… 20.04. Bumped swift-nio and swift-collections versions on manifest
@NeedleInAJayStack
Copy link
Member

Thanks @3a4oT Just wondering - Are there any particular needs you had for upgrading this, or were you just cleaning it up?

Comment on lines -10 to +11
.package(url: "https://github.com/apple/swift-nio.git", .upToNextMajor(from: "2.10.1")),
.package(url: "https://github.com/apple/swift-collections", .upToNextMajor(from: "1.0.0")),
.package(url: "https://github.com/apple/swift-nio.git", .upToNextMajor(from: "2.70.0")),
.package(url: "https://github.com/apple/swift-collections", .upToNextMajor(from: "1.0.3")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why narrow these ranges? Wouldn't updating them in the Package.resolved be enough if we aren't using any new features introduced? Or does Swift package manager require that we pin to a compatible swift-tools-version?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason is: Why not support the latest and greatest? :) As I understand it, it's not a breaking change in terms of API and contains important fixes and optimizations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not support the latest and greatest?

Okay, but the last one already supported the latest and greatest. For example, both the old and new Package.swift would allow swift-nio v2.72.0. What you've done here only says that we are no longer compatible with, say, v2.68.0, which will require every downstream package to bump its NIO dependency to v2.70.0 or above. It's fine to bump this minimum version if we are relying on new functionality introduced, but if our library is functionally compatible with old versions we should allow them.

it's not a breaking change in terms of API and contains important fixes and optimizations.

While it's true that it contains important fixes and optimizations, the downstream package is in a better position to decide if and when they need them.

My request is that we remove these changes to Package.swift. We can keep the changes to Package.resolved so that we are testing the latest versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @NeedleInAJayStack!

@NeedleInAJayStack
Copy link
Member

@paulofaria Do you have any thoughts on this? I know you've been hesitant to update swift-tools versions in the past, but NIO is at 5.8 compatibility

@3a4oT
Copy link
Author

3a4oT commented Sep 5, 2024

Thanks @3a4oT Just wondering - Are there any particular needs you had for upgrading this, or were you just cleaning it up?

Hello, thanks for asking. The main driver was that we should support what Swift-NIO supports regarding the minimum required Swift tools. Swift itself doesn't support Ubuntu 20.04 anymore, so I dropped 5.4-5.6. I thought that bumping the version to 5.8 would be too aggressive, but I still believe it's the right thing to do :)

@NeedleInAJayStack
Copy link
Member

Hey @3a4oT, I think this is good to go if you just make the following changes:

  1. Remove dependency changes in Package.swift
  2. Run the formatter: docker run --rm -v ./:/repo ghcr.io/nicklockwood/swiftformat:latest /repo

Let me know if you need any help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants